CKKSVector Polynomial Evaluation #99
Conversation
youben11
added
the
Type: New Feature ➕
youben11
added this to the Training and Evaluation of Logistic Regression on Encrypted Data milestone
tenseal/tensors/ckksvector.cpp
Outdated
| "the coefficients vector need to have at least one element"); | ||
| } | ||
|
|
||
| int degree = coefficients.size() - 1; |
There was a problem hiding this comment.
isn't this generating a warning? I think it should be
static_cast<int>(coefficients.size())
just to keep it a signed integer.
There was a problem hiding this comment.
My confusion here is that coefficients.size() returns size_t, which is an unsigned int64.
Assigning to int should raise some warnings from any compiler, I think.
And you can use
int degree = static_cast<int>(coefficients.size()) - 1;
to prevent the warnings.
There was a problem hiding this comment.
Ah yeah of course, didn't saw that I didn't cast, thank you!
tenseal/tensors/ckksvector.cpp
Outdated
| // pre-compute squares of x | ||
| CKKSVector x = *this; | ||
| int max_square = static_cast<int>(floor(log2(degree))); | ||
| cout << "max square " << max_square << endl; |
There was a problem hiding this comment.
Do we need stdout logging here?
| int max_square = static_cast<int>(floor(log2(degree))); | ||
| cout << "max square " << max_square << endl; | ||
| vector<CKKSVector> x_squares; | ||
| x_squares.reserve(max_square + 1); |
There was a problem hiding this comment.
Why not directly resize? and access the vector directly using [] instead of push_back operations
There was a problem hiding this comment.
Update: So it's a little complicated for this, reserve is more convenient for our usage, as resize need to init the objects in the vector, and we doesn't want this, however, reserve will only allocate the necessary memory and push_back won't have to allocate memory dynamically.
There was a problem hiding this comment.
Yeah, my observation made more sense with vector of pointers.
Sorry about that.
| } | ||
|
|
||
| // set result accumulator to the constant coefficient | ||
| vector<double> cst_coeff(this->size(), coefficients[0]); |
There was a problem hiding this comment.
this creates a vector of size this->size() with all the values set to coefficients[0]. is that the expected behavior?
There was a problem hiding this comment.
yep, to encrypt it later and have operations done element-wise
tenseal/tensors/ckksvector.cpp
Outdated
| // pre-compute squares of x | ||
| CKKSVector x = *this; | ||
| int max_square = static_cast<int>(floor(log2(degree))); | ||
| vector<CKKSVector> x_squares(max_square + 1); |
There was a problem hiding this comment.
I think the build fails here now, because it tries to call CKKSVector with the default constructor.
If
vector<CKKSVector> x_squares;
x_squares.resize(max_square + 1);
doesn't work, ignore this, my bad.
* make it work with tests * log(n) mul depth for polynomial of degree n * fix some tests * rename tests * some fixes and new tests * case where poly is of degree < 2 * more tests * rescale the encrypt_zero ciphertext * null polynomial tests * remove debug * fix _mul_plain_inplace, add some tests * optimize circuit considering coeffecient * balancing multiplication circuit * more tests, docs and TODO * more tests * fix bugs, refactor and simplify algorithm * simplify accumulator creation * move compute_polynomial_term to utils * remove inclue <map> * lint * requested changes * cast * fall back to reserve Co-authored-by: philomath213 <bilalphilomath@gmail.com>
Description
This PR introduce a new method for CKKSVector to be able to evaluate a polynomial over all the vector values (
encrypted_vector.polyval(list_of_polynomial_coefficients))Fixes #95
How has this been tested?
Checklist